fix(multichain-account-service): prevent spurious alignment logs + smaller map state updates#8136
fix(multichain-account-service): prevent spurious alignment logs + smaller map state updates#8136
Conversation
ccharly
force-pushed
the
cc/fix/align-accounts-logs
branch
from
5fe9be0 to
db3a2cc
Compare
| return { wrapper, innerProvider }; | ||
| } | ||
|
|
||
| describe('AccountProviderWrapper', () => { |
There was a problem hiding this comment.
I've added some more tests for the wrapper, since we skip the alignAccounts call from the wrapper entirely now in case it's disabled.
So, with this new file, we can actually get better coverage in a more "unit-tests-like way" for this class.
| displayName, | ||
|
|
||
| // Exclude test helpers from coverage collection | ||
| coveragePathIgnorePatterns: ['.*/src/tests/.*'], |
There was a problem hiding this comment.
Some of the mock providers are not covered/used anymore by our tests.
The src/tests/providers.ts file is pretty messy right now, and instead of removing/moving things around, I prefer to not have it in the coverage at all for the time being.
I do plan to make a small refactor around that to avoid the .prototype hacks we're using there.
ccharly
changed the title
…maller map updates
ccharly
force-pushed
the
cc/fix/align-accounts-logs
branch
from
db3a2cc to
875cf51
Compare
| } from '../tests'; | ||
| import type { MultichainAccountServiceMessenger } from '../types'; | ||
|
|
||
| class MockInnerProvider extends BaseBip44AccountProvider { |
There was a problem hiding this comment.
| class MockInnerProvider extends BaseBip44AccountProvider { | |
| class MockWrappedProvider extends BaseBip44AccountProvider { |
|
We can actually close this now, the |
2 participants
Explanation
The current condition was ALWAYS logging the
Found missing accounts for account provider...line, since all alignments always returns the full list of accounts (even existing ones).To avoid triggering this, we now make a proper diff-set between the old accounts and new ones.
There's another benefit with doing this. Instead of clearing the entire state map, we now make smaller state update, which should be slightly faster when we have a tons of groups with a lot of accounts.
References
N/A
Checklist
Note
Medium Risk
Touches
MultichainAccountGroup.alignAccountsstate-mapping logic, including removing accounts when providers return fewer IDs, which could impact account visibility if the diffing is incorrect. Changes are scoped and covered by new unit tests, lowering the likelihood of regressions.Overview
Improves
MultichainAccountGroup.alignAccountsby diffing old vs new account IDs per provider instead of clearing and rebuilding all provider/account maps on every alignment. This prevents the “missing accounts” log from firing on no-op alignments and reduces state churn by only updating mappings when accounts were actually added/removed (and deleting mappings when a provider becomes disabled).Adds tests to ensure alignment removes accounts that providers stop returning, and introduces
AccountProviderWrapperalignment tests for enabled/disabled behavior. Updates Jest config to ignoresrc/testshelpers for coverage, and records the change/fix in the package changelog.Written by Cursor Bugbot for commit 875cf51. This will update automatically on new commits. Configure here.